Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate to eslint #69

Merged
merged 6 commits into from
Oct 9, 2019
Merged

Migrate to eslint #69

merged 6 commits into from
Oct 9, 2019

Conversation

raajnadar
Copy link
Contributor

Reason for PR

palantir/tslint#4534

What did I do?

  • Added eslint and prettier config.
  • Fixed lint issues.
  • Added npm script to run lint test with CI.

While setting up I found few typescript warning which we need to fix.

abhinavdalal-iconnect
abhinavdalal-iconnect reviewed Oct 7, 2019
View reviewed changes
Copy link
Contributor

@abhinavdalal-iconnect abhinavdalal-iconnect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me!

@raajnadar
Copy link
Contributor Author

raajnadar commented Oct 9, 2019 •
edited
Loading

@YIZHUANG please review this pr.. This pr will make the development more easier once this is merge I will send another pr for vscode and editor config.

We can configure preetier according to our needs initially the project used double quotes but I changed to single.

@raajnadar
Copy link
Contributor Author

I suppressed all the warnings which we can fix in future updates most of they are due to the usage of any keyword in typings.

We are left with 2 errors which I tried to fix but have no idea how to fix it.

YIZHUANG
YIZHUANG requested changes Oct 9, 2019
View reviewed changes
package.json Outdated Show resolved Hide resolved
@YIZHUANG
Copy link
Owner

YIZHUANG commented Oct 9, 2019 •
edited
Loading

Thank you for the PR.
The "eslint src/**/*{.js,.jsx,.ts,.tsx}" doesn't run and husky is lacking. Otherwise looks good.

@raajnadar
Copy link
Contributor Author

I went through the package.json and found that husky is not configured I will create a separate PR with husky pre-commit hook and all the necessary commands.

I think we should not make this PR more complicated..!

@YIZHUANG YIZHUANG merged commit 43a4098 into YIZHUANG:master Oct 9, 2019
@raajnadar raajnadar deleted the migrate-to-eslint branch October 9, 2019 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abhinavdalal-iconnect abhinavdalal-iconnect abhinavdalal-iconnect left review comments

@YIZHUANG YIZHUANG YIZHUANG approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@raajnadar @YIZHUANG @abhinavdalal-iconnect